gh-109532: fix socket HOWTO inaccuracy about send() on broken connection#144747
gh-109532: fix socket HOWTO inaccuracy about send() on broken connection#144747Gh-Novel wants to merge 2 commits intopython:mainfrom
Conversation
…onnection Correct the claim that send() returns 0 bytes on a broken connection. In practice, send() on a broken connection raises OSError (EPIPE) rather than returning 0. Only recv() returns 0 bytes to indicate disconnection. Add a clarifying comment to the mysend example noting this distinction.
bkap123
left a comment
There was a problem hiding this comment.
I think that these are good (and important) changes. I think that the difference between how send and recv work for a closed connection could be better emphasized (see comment).
Doc/howto/sockets.rst
Outdated
| that *there is no* :abbr:`EOT (End of Transfer)` *on a socket.* I repeat: if a socket | ||
| ``send`` or ``recv`` returns after handling 0 bytes, the connection has been | ||
| broken. If the connection has *not* been broken, you may wait on a ``recv`` | ||
| that *there is no* :abbr:`EOT (End of Transfer)` *on a socket.* I repeat: if a |
There was a problem hiding this comment.
These changes look good.
The one suggestion I have is to emphasize that calling send on a closed socket results in an OSError and thus should not actually happen in real code. In contrast, recv returning 0 for a closed socket is a feature. There is a paragraph above that explains what a return value of 0 from recv means, but I think there could be a better explanation of return values/exceptions forsend. Possibly say: In contrast, you should never call send on a broken socket, as it results in an OSError.
Doc/howto/sockets.rst
Outdated
| that *there is no* :abbr:`EOT (End of Transfer)` *on a socket.* I repeat: if a socket | ||
| ``send`` or ``recv`` returns after handling 0 bytes, the connection has been | ||
| broken. If the connection has *not* been broken, you may wait on a ``recv`` | ||
| that *there is no* :abbr:`EOT (End of Transfer)` *on a socket.* I repeat: if a |
There was a problem hiding this comment.
| that *there is no* :abbr:`EOT (End of Transfer)` *on a socket.* I repeat: if a | |
| that *there is no* :abbr:`EOT (End of Transfer)` *on a socket.* If a |
There was a problem hiding this comment.
Thank you for the review! I've addressed both suggestions - removed "I repeat:" and restructured the paragraph to better emphasize that send() on a broken socket raises OSError (an error you should handle), in contrast to recv() returning 0 bytes as a deliberate disconnect signal.
Remove informal 'I repeat:' phrasing. Restructure paragraph to clearly contrast recv() returning 0 bytes (a deliberate signal of disconnection) with send() raising OSError (an error condition — should never be called on a broken socket). Co-authored-by: bkap123 <[email protected]>
bkap123
left a comment
There was a problem hiding this comment.
Left one more suggestion, but otherwise LGTM!
Also as a reminder, if happen to be using an LLM, just remember these rules.
| @@ -201,6 +203,8 @@ length message:: | |||
| sent = self.sock.send(msg[totalsent:]) | |||
| if sent == 0: | |||
There was a problem hiding this comment.
I actually think that we can get rid of this if clause and the RuntimeError exception since send will already raise an error on a broken connection. That would also make more sense because your comment below is basically saying that send raises an error automatically so there is no reason to check its return value.
socket.sendbehavior #109532Summary
Corrects an inaccuracy in the Socket Programming HOWTO (
Doc/howto/sockets.rst) aboutsend()behavior when the remote end has disconnected.The existing text stated "if a socket
sendorrecvreturns after handling 0 bytes, the connection has been broken." In practice,send()on a broken connection raisesOSError(specificallyBrokenPipeError/EPIPE) rather than returning 0. Onlyrecv()returns 0 bytes to indicate disconnection.Changes
send()(raisesOSError) andrecv()(returns 0 bytes)mysendcode exampleTesting
send()raisesBrokenPipeErrorandrecv()returnsb''📚 Documentation preview 📚: https://cpython-previews--144747.org.readthedocs.build/